Add ZSTD bindings to the node:zlib package. by alistairjevans · Pull Request #6007 · cloudflare/workerd

alistairjevans · 2026-02-03T11:09:01Z

Based pretty heavily on the brotli support.

Closes #4013

github-actions · 2026-02-03T11:09:14Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

alistairjevans · 2026-02-03T11:09:48Z

I have read the CLA Document and I hereby sign the CLA

src/workerd/api/node/zlib-util.c++

jasnell · 2026-02-03T15:14:31Z

src/workerd/api/node/zlib-util.c++

+ZstdEncoderContext::~ZstdEncoderContext() {
+  if (cctx_ != nullptr) {
+    ZSTD_freeCCtx(cctx_);
+    cctx_ = nullptr;


It might be worth wrapping this in a custom smart pointer to ensure the cleanup is correct. Non-blocking tho.

Added (brotli implementation does already do this)

src/workerd/api/node/zlib-util.c++

jasnell · 2026-02-03T15:25:30Z

@anonrig ... given your familiarity with the zlib impl in workerd, it would be good to have your review on this as well.

src/node/internal/internal_zlib.ts

anonrig · 2026-02-03T15:56:32Z

src/node/internal/internal_zlib_base.ts

+  constructor(options: ZstdOptions | undefined | null, mode: number) {
+    ok(mode === CONST_ZSTD_DECODE || mode === CONST_ZSTD_ENCODE);
+    zstdInitParamsArray.fill(-1);


This diverges from the Node.js implementation and makes it a lot less readable. Ideally the constructor should receive a parameter for initParamsArray just like Node.js

class Zstd extends ZlibBase { constructor(opts, mode, initParamsArray, maxParam) {

If we diverge from Node.js, finding bugs & keep the code in sync would be a lot harder then it really is.

Updated 👍

anonrig · 2026-02-03T15:57:26Z

src/workerd/api/node/tests/BUILD.bazel

+    size = "large",
+    src = "zstd-nodejs-test.wd-test",
+    args = ["--experimental"],
+    data = ["zstd-nodejs-test.js"],


Suggested change

data = ["zstd-nodejs-test.js"],

data = ["zlib-zstd-nodejs-test.js"],

anonrig · 2026-02-03T15:57:32Z

src/workerd/api/node/tests/BUILD.bazel


+wd_test(
+    size = "large",
+    src = "zstd-nodejs-test.wd-test",


Suggested change

src = "zstd-nodejs-test.wd-test",

src = "zlib-zstd-nodejs-test.wd-test",

src/workerd/api/node/tests/zstd-nodejs-test.js

anonrig · 2026-02-03T16:00:04Z

src/workerd/api/node/tests/zstd-nodejs-test.js

+export const zstdConstantsTest = {
+  test() {
+    // Flush directives
+    assert.strictEqual(typeof zlib.constants.ZSTD_e_continue, 'number');


You've already tested this in zlib-nodejs-test.

Removed duplicate

src/node/internal/internal_zlib.ts

alistairjevans · 2026-02-03T16:54:07Z

Thank you for the feedback @anonrig and @jasnell! 🙏 I believe I've addressed all your comments now.

alistairjevans · 2026-02-11T20:56:56Z

Merged latest; @anonrig / @jasnell , any chance of a re-review? 🙏

anonrig · 2026-02-11T21:26:28Z

LGTM. @alistairjevans formatter seems to be failing: https://github.com/cloudflare/workerd/actions/runs/21922721360/job/63307147452?pr=6007

@jasnell can you take a look?

alistairjevans · 2026-02-11T21:49:35Z

Thanks @anonrig, formatting fix pushed, linter passes locally now.

jasnell · 2026-02-11T22:25:49Z

Overall I think this looks good. What I'm considering... (something I wish I had thought to do on the previous node:zlib additions)... is adding this initially behind an experimental compat flag and having the security team do some quick fuzz testing on it. Anything new that is dealing with bytes being passed around makes me just a bit nervous and a little extra testing doesn't hurt. That's not to say I see anything in here concretely questionable, just thinking about taking the extra precaution. @danlapid what do you think?

alistairjevans · 2026-02-12T14:56:29Z

@jasnell, would such a compatibility flag still allow us to opt-in to using the feature in Cloudflare Workers while we wait for the fuzz test to complete?

jasnell · 2026-02-12T15:02:28Z

@alistairjevans ... while the fuzz testing would happening (if we go that route) the compat flag would be marked as "experimental" which means only cloudflare accounts would be able to use it until that annotation is removed. Still need to decide tho if this is the path we'd want to take. Should have a decision soon.

danlapid · 2026-02-12T15:15:54Z

Let's not block - good idea to do as follow up - still want to see this land.
Let's review the code until we're confident this is safe.

jasnell

Overall, LGTM with a couple nits and a question about error handling that might need addressing before this lands. I'll do the sign off once the question is looked at :-) .. great job overall!!

There is a pre-existing issue we need to address with CompressionStream::write's validation of flush values but that's not introduced or made worse with this change (it's relevant for the brotli support also). And, there's missing external memory tracking here that can be done as a follow-up (see, for instance, the CompressionAllocator::AllocForBrotli, CompressionAllocator::FreeForZlib, etc). We'll want to come back to that.

jasnell · 2026-02-12T15:34:57Z

src/workerd/api/node/zlib-util.c++

+}
+
+kj::Maybe<CompressionError> ZstdDecoderContext::resetStream() {
+  if (dctx_.get() != nullptr) {


Should this be clearing the error_ state? e.g. if work() fails, then resetStream() is called, then work() succeeds, will getError() be returning the right thing?

jasnell · 2026-02-12T15:35:40Z

src/workerd/api/node/zlib-util.c++

+}
+
+kj::Maybe<CompressionError> ZstdDecoderContext::setParams(int key, int value) {
+  size_t result = ZSTD_DCtx_setParameter(dctx_.get(), static_cast<ZSTD_dParameter>(key), value);


Nit: should there be a KJ_DASSERT to verify dctx_.get() is valid?

jasnell · 2026-02-12T15:36:16Z

src/workerd/api/node/zlib-util.c++

+  // For decompression, lastResult == 0 means frame is complete
+  // If we have flush_ == ZSTD_e_end equivalent and there's input left, that's an error
+  if (flush_ == ZSTD_e_end && input_.pos < input_.size && lastResult == 0) {
+    // Frame completed but there's still input - could be trailing data


Nit: what is this if block for if it's empty?

…a sync callback

Add ZSTD bindings to the node:zlib package.

ab0e269

alistairjevans requested review from a team as code owners February 3, 2026 11:09

github-actions bot added a commit that referenced this pull request Feb 3, 2026

@alistairjevans has signed the CLA in #6007

87f4359

alistairjevans mentioned this pull request Feb 3, 2026

node:zlib implement zstd #4013

Open

jasnell reviewed Feb 3, 2026

View reviewed changes

src/workerd/api/node/zlib-util.c++ Outdated Show resolved Hide resolved

jasnell reviewed Feb 3, 2026

View reviewed changes

src/workerd/api/node/zlib-util.c++ Show resolved Hide resolved

jasnell reviewed Feb 3, 2026

View reviewed changes

src/workerd/api/node/zlib-util.c++ Outdated Show resolved Hide resolved

jasnell reviewed Feb 3, 2026

View reviewed changes

src/workerd/api/node/zlib-util.c++ Outdated Show resolved Hide resolved

jasnell reviewed Feb 3, 2026

View reviewed changes

src/workerd/api/node/zlib-util.c++ Outdated Show resolved Hide resolved

jasnell reviewed Feb 3, 2026

View reviewed changes

src/workerd/api/node/zlib-util.c++ Outdated Show resolved Hide resolved

jasnell requested a review from anonrig February 3, 2026 15:25

anonrig reviewed Feb 3, 2026

View reviewed changes

alistairjevans added 6 commits February 3, 2026 16:20

zlib-util minor formatting and asserts

e1da60c

Bring zstd arguments in line with nodejs layout.

3e2357e

Test updates

3f9e786

Add zstdCheckError helper function.

08acff0

Remove duplicating test

915c17a

Use smart pointer for zstd context.

2295317

Merge branch 'main' into alistairjevans/zstd-support

8d66963

anonrig approved these changes Feb 11, 2026

View reviewed changes

Fix formatting.

e77c44a

jasnell reviewed Feb 12, 2026

View reviewed changes

Fix bug where large files with many chunks would stack-overflow with …

395ce8a

…a sync callback

	data = ["zstd-nodejs-test.js"],
	data = ["zlib-zstd-nodejs-test.js"],

	src = "zstd-nodejs-test.wd-test",
	src = "zlib-zstd-nodejs-test.wd-test",

Conversation

alistairjevans commented Feb 3, 2026

Uh oh!

github-actions bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alistairjevans commented Feb 3, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jasnell commented Feb 3, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alistairjevans commented Feb 3, 2026

Uh oh!

alistairjevans commented Feb 11, 2026

Uh oh!

anonrig commented Feb 11, 2026

Uh oh!

alistairjevans commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasnell commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alistairjevans commented Feb 12, 2026

Uh oh!

jasnell commented Feb 12, 2026

Uh oh!

danlapid commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasnell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Feb 3, 2026 •

edited

Loading

alistairjevans commented Feb 11, 2026 •

edited

Loading

jasnell commented Feb 11, 2026 •

edited

Loading

danlapid commented Feb 12, 2026 •

edited

Loading